Skip to content

Conversation

@freke
Copy link
Owner

@freke freke commented Sep 23, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Authentication now treats unknown client types safely: logs a warning and defaults to a regular client instead of failing.
  • Chores

    • CI workflow renamed and refocused on Docker image publishing; tag push trigger removed.
    • Docker build enhanced with automated multi-format tagging (branch, semver variants).
    • Docker release action and base runtime updated; builds now use generated tags automatically.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 23, 2025

Walkthrough

Updated GitHub Actions release workflow to generate Docker metadata tags and switch the beam-docker-release action; TCP auth handling now maps 0→regular, 1→bot, and logs+defaults to regular for unexpected values.

Changes

Cohort / File(s) Summary of changes
Release workflow updates
.github/workflows/release.yml
Renamed workflow; removed tag-based push trigger; added docker/metadata-action@v5 to emit multi-format tags; switched shortishly/beam-docker-release-action@v1freke/beam-docker-release-action@main; added build-image: erlang:latest; use ${{ steps.meta.outputs.tags }} for build-tags.
TCP auth type handling
src/erlmur_tcp_message.erl
In Authenticate handling, map 0regular, 1bot; add catch-all branch that logs a warning for unexpected Type values and defaults to regular. No exported/public signatures changed.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant S as TCP Server
  participant L as Logger
  rect rgba(200,230,255,0.3)
    C->>S: Authenticate(Type)
    alt Type == 0
      S->>S: Map Type to "regular"
    else Type == 1
      S->>S: Map Type to "bot"
    else Unknown
      S->>L: Warn about unexpected auth Type
      S->>S: Default mapping to "regular"
    end
    S-->>C: Authentication response (role)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Github workflow #4 — Modifies the same GitHub Actions release workflow and addresses beam-docker-release action usage.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Workfow updates" is short and directly related to the main change (updates to the CI/workflow file), so it reflects the primary intent of the changeset; however it is generic and contains a typo which reduces clarity for readers.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch workfow_updates

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
src/erlmur_tcp_message.erl (1)

228-230: Authenticate.client_type fallback looks good; consider explicit 0 and logging unknowns.

Clearer intent and easier debugging if 0 is handled explicitly and unknown values are logged while still defaulting to regular.

-            1 -> bot;
-            _ -> regular
+            1 -> bot;
+            0 -> regular;
+            _ ->
+                logger:debug("Unknown Authenticate.client_type ~p; defaulting to regular", [Type]),
+                regular
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1d8fb5 and 40d3c51.

⛔ Files ignored due to path filters (1)
  • devenv.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • .github/workflows/release.yml (2 hunks)
  • src/erlmur_tcp_message.erl (1 hunks)
🔇 Additional comments (1)
.github/workflows/release.yml (1)

1-1: Rename LGTM.

No issues with the workflow name change.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40d3c51 and e6a568b.

📒 Files selected for processing (1)
  • .github/workflows/release.yml (2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/release.yml

24-24: undefined function "replace". available functions are "always", "cancelled", "contains", "endswith", "failure", "format", "fromjson", "hashfiles", "join", "startswith", "success", "tojson"

(expression)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: build
🔇 Additional comments (3)
.github/workflows/release.yml (3)

1-1: Workflow rename LGTM

Clear, accurate name.


16-16: Pin the action ref; avoid @main for supply‑chain safety and reproducibility

Use a released tag or commit SHA.

-        uses: freke/beam-docker-release-action@main
+        uses: freke/beam-docker-release-action@<tag-or-commit-sha>
+        # Example:
+        # uses: freke/beam-docker-release-action@v1.2.3
+        # or pin to a commit SHA:
+        # uses: freke/beam-docker-release-action@<sha>

21-21: Avoid erlang:latest; pin a specific version (ideally with a digest) for deterministic builds

Mutable tags can break builds unexpectedly.

-          build-image: erlang:latest
+          build-image: erlang:27
+          # For full reproducibility, prefer a digest:
+          # build-image: erlang:27@sha256:<digest>

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
.github/workflows/release.yml (2)

15-17: Harden Docker tag sanitization (allowable charset, length, fallback).
Current step only replaces “/”. Add lowercasing, full charset filtering, 128‑char limit, and a safe fallback.

-      - name: Sanitize ref_name for Docker tag
-        run: |
-          echo "SANITIZED_REF=${GITHUB_REF_NAME//\//-}" >> "$GITHUB_ENV"
+      - name: Sanitize ref_name for Docker tag
+        run: |
+          RAW="${GITHUB_REF_NAME}"
+          # lowercase, replace invalid chars with '-', trim leading/trailing '-', limit to 128, fallback to sha
+          SANITIZED="$(echo -n "${RAW}" \
+            | tr '[:upper:]' '[:lower:]' \
+            | sed -E 's/[^a-z0-9._-]+/-/g; s/^-+//; s/-+$//' \
+            | cut -c1-128)"
+          if [ -z "$SANITIZED" ]; then SANITIZED="sha-${GITHUB_SHA:0:7}"; fi
+          echo "SANITIZED_REF=$SANITIZED" >> "$GITHUB_ENV"

27-27: Using SANITIZED_REF for build-tags is correct. Consider adding a ‘latest’ tag on main.
Optional: publish both the branch tag and latest when building from main.

Would you like me to propose a small change to emit an extra latest tag only on main?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e6a568b and a4403c2.

📒 Files selected for processing (1)
  • .github/workflows/release.yml (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: build
🔇 Additional comments (3)
.github/workflows/release.yml (3)

1-1: Workflow name change looks good.
Clearer title for what the job does.


24-24: Avoid erlang:latest; pin a specific version (ideally with a digest).
Prevents surprise breakages and ensures reproducibility.

-          build-image: erlang:latest
+          build-image: erlang:27
+          # or pin by digest for full reproducibility:
+          # build-image: erlang:27@sha256:<digest>

19-19: Pin the action ref; avoid @main for supply‑chain safety.
Use a released tag or a commit SHA.

-        uses: freke/beam-docker-release-action@main
+        uses: freke/beam-docker-release-action@<tag-or-commit-sha>
#!/bin/bash
# Fetch available tags to pick a stable one to pin.
gh api /repos/freke/beam-docker-release-action/tags -q '.[].name' | head -n 10

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f06ef9 and 4189e24.

📒 Files selected for processing (2)
  • .github/workflows/release.yml (1 hunks)
  • src/erlmur_tcp_message.erl (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/erlmur_tcp_message.erl
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: build
🔇 Additional comments (4)
.github/workflows/release.yml (4)

1-1: LGTM: Workflow rename is fine.


29-29: Pin the action ref; avoid @main for supply‑chain safety.

Pin to a release tag or commit SHA:

-        uses: freke/beam-docker-release-action@main
+        uses: freke/beam-docker-release-action@<tag-or-commit-sha>

34-34: Avoid erlang:latest; pin a specific version (ideally by digest) for deterministic builds.

Example:

-          build-image: erlang:latest
+          build-image: erlang:27
+          # or fully pinned:
+          # build-image: erlang:27@sha256:<digest>

37-37: No change required — comma-separated tags are supported

docker/metadata-action's sep-tags can emit comma-separated tags; docker/build-push-action accepts CSV (comma‑delimited) tags and freke/beam-docker-release-action simply forwards build-tags to build-push-action, so sep-tags: ',' is compatible.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
.github/workflows/release.yml (3)

16-27: Emit a literal “latest” tag only on semver tags

docker/metadata-action won’t produce a raw “latest” without an explicit raw rule. Gate it to semver to avoid “latest” on non-releases.

           tags: |
             type=ref,event=branch
-            type=ref,event=pr
             type=semver,pattern={{version}}
             type=semver,pattern={{major}}.{{minor}}
             type=semver,pattern={{major}}
+            # publish literal latest only on semver tags
+            type=raw,value=latest,enable={{is_semver}}

16-27: The PR tag rule is ineffective (workflow doesn’t run on pull_request)

Either remove it (shown above) or add a pull_request trigger if you truly want PR images.

Option to enable PR builds:

 on:
-  push:
+  push:
+  pull_request:

22-22: Reduce registry churn: restrict branch tags to default branch (optional)

Prevents publishing images for every feature branch push.

-            type=ref,event=branch
+            type=ref,event=branch,enable={{is_default_branch}}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b54b267 and 285b755.

📒 Files selected for processing (1)
  • .github/workflows/release.yml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: build
🔇 Additional comments (3)
.github/workflows/release.yml (3)

27-27: Confirm separator compatibility between metadata outputs and the Beam action

You set sep-tags to comma. Ensure the beam-docker-release action splits on commas; many tools expect newline-separated tags. If unsure, drop sep-tags to use the default newline-separated output.

-          sep-tags: ','
+          # Use default (newline) separation to maximize compatibility
+          # sep-tags: ','

Also applies to: 37-37


29-29: Pin the action ref; avoid using @main for supply‑chain safety and reproducibility

Use a released tag or a commit SHA.

-        uses: freke/beam-docker-release-action@main
+        uses: freke/beam-docker-release-action@<tag-or-commit-sha>

34-34: Avoid erlang:latest; pin a specific OTP version (ideally with a digest)

Latest is mutable and can break builds.

-          build-image: erlang:latest
+          build-image: erlang:27
+          # Or pin by digest for full reproducibility:
+          # build-image: erlang:27@sha256:<digest>

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
.github/workflows/release.yml (1)

16-26: Emit a literal latest tag gated to semver tags.

Add a raw latest tag only when the ref is a valid semver tag.

           tags: |
             type=ref,event=branch
             type=semver,pattern={{version}}
             type=semver,pattern={{major}}.{{minor}}
             type=semver,pattern={{major}}
+            # publish "latest" only for semver tag pushes
+            type=raw,value=latest,enable={{is_semver}}

Also confirm you intend to publish images for every branch (type=ref,event=branch). If not, restrict the workflow to tags or drop the branch tag line.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 285b755 and a5b3c18.

📒 Files selected for processing (1)
  • .github/workflows/release.yml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: build
🔇 Additional comments (3)
.github/workflows/release.yml (3)

33-33: Avoid erlang:latest; pin a specific version (or digest) for deterministic builds.

-          build-image: erlang:latest
+          build-image: erlang:27
+          # or fully pinned:
+          # build-image: erlang:27@sha256:<digest>

36-36: Verify build-tags format compatibility with the action.

You set sep-tags to ','; ensure freke/beam-docker-release-action accepts a comma-separated list. If it expects newline-separated tags, either drop sep-tags or split before passing.

Option A — use default newline-separated output:

-          build-tags: ${{ steps.meta.outputs.tags }}
+          build-tags: ${{ steps.meta.outputs.tags }}

(and remove sep-tags: ',' above)

Option B — keep comma-separated but confirm support in the action’s docs/repo. If unsure, I can look it up and suggest the exact expected format.


28-28: Pin the action ref; avoid @main for supply‑chain safety and reproducibility.

-        uses: freke/beam-docker-release-action@main
+        uses: freke/beam-docker-release-action@<tag-or-commit-sha>
+        # e.g. @v1.2.3 or @<sha>; document and update periodically

@freke freke merged commit f528462 into main Sep 24, 2025
4 checks passed
@freke freke deleted the workfow_updates branch September 24, 2025 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant